W-17440369: Use instanceUrl host when refreshing OAuth tokens#2936
Open
wmathurin wants to merge 2 commits into
Open
W-17440369: Use instanceUrl host when refreshing OAuth tokens#2936wmathurin wants to merge 2 commits into
wmathurin wants to merge 2 commits into
Conversation
Adds OAuth2.overrideLoginServerIfNeeded() with precedence: communityUrl > instanceServer > loginServer instanceServer is null until after the first token response, so the code-exchange path naturally falls through to loginServer with no special casing. Updates both token-refresh call sites (ClientManager, AuthenticatorService) to use the helper, and updates getOpenIDToken() to accept and apply the same precedence. Adds a target-host log line at each refresh call site. Mirrors the iOS fix from PR #4074 (SFOAuthCredentials.overrideDomainIfNeeded). Tests: 6 new unit tests in OAuth2OverrideLoginServerTests covering precedence, fallback, community precedence, code-exchange neutrality, and malformed-URL fallback.
Generated by 🚫 Danger |
MockK's relaxed mocks return "" (not null) for unstubbed Java String
getters. Guard against empty strings in the same way as null to prevent
new URI("") from constructing a relative URI that breaks OkHttp.
Also adds a 7th test covering the empty-instanceServer case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Token refresh previously always POSTed to
loginServer(e.g.login.salesforce.com). This breaks DPoPhtubinding (RFC 9449 §4.3) when the login-pool redirects to the user's My Domain, and adds an unnecessary round-trip on every refresh for Bearer flows.Mirrors the iOS fix from PR #4074.
Changes
OAuth2.javaoverrideLoginServerIfNeeded(loginServer, instanceServer, communityId, communityUrl)static helper with precedence:communityUrl > instanceServer > loginServer.instanceServeris null until after the first token response, so the code-exchange path falls through tologinServernaturally with no special-casing.getOpenIDToken()to accept and apply the same precedence (new parameters:instanceServer,communityId,communityUrl).ClientManager.java—AccMgrAuthTokenProvider.refreshStaleToken()overrideLoginServerIfNeededbefore callingrefreshAuthTokenAuthenticatorService.java—Authenticator.getAuthToken()OAuth2OverrideLoginServerTests.kt— 6 new unit tests:OAuth2Test.java— UpdatedtestGetOpenIDTokencall to match newgetOpenIDTokensignature.Test plan
OAuth2OverrideLoginServerTestspass on emulator (API 36)OAuth2MockTests(6 tests) passLoginServerManagerTestandScopeParserTestpass